Conversation
📝 WalkthroughWalkthrough리뷰 생성 시 그룹바이 포스트와 판매자 평점에 가중평균(EMA) 방식으로 기여도를 반영하도록 새 엔티티·필드·서비스와 비관적 잠금을 도입했습니다. 리뷰 생성 흐름을 ApplicationService로 이동해 여러 서비스 간 트랜잭션을 조정합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as ReviewController
participant AppSvc as ReviewApplicationService
participant OrderSvc as OrderService
participant ReviewSvc as ReviewService
participant GroupSvc as GroupBuyService
participant UserSvc as UserService
participant DB as Database
Client->>Controller: POST /reviews (writerId, orderId, score)
Controller->>AppSvc: createReview(writerId, request)
AppSvc->>OrderSvc: validate & get Order
OrderSvc-->>AppSvc: Order
AppSvc->>ReviewSvc: createReviewEntity(writerId, Order, score)
ReviewSvc->>DB: INSERT Review
DB-->>ReviewSvc: reviewId
ReviewSvc-->>AppSvc: reviewId
AppSvc->>GroupSvc: addPostRating(postId, score)
GroupSvc->>DB: select GroupBuyPost for update (pessimistic lock)
DB-->>GroupSvc: GroupBuyPost
GroupSvc->>GroupSvc: post.addRating(score) // update ratingSum/count/avg
GroupSvc->>DB: save GroupBuyPost
AppSvc->>GroupSvc: getPostWithLock(postId)
GroupSvc->>DB: select GroupBuyPost (pessimistic lock)
DB-->>GroupSvc: updated GroupBuyPost
AppSvc->>GroupSvc: countPostsByLeader(sellerId)
GroupSvc-->>AppSvc: postCount
AppSvc->>UserSvc: applyPostContribution(sellerId, postId, postAvg, reviewCount, postCount)
UserSvc->>DB: select User for update (pessimistic lock)
DB-->>UserSvc: User
UserSvc->>DB: select/create SellerPostRatingContribution for update
DB-->>UserSvc: SellerPostRatingContribution
UserSvc->>UserSvc: computeAlpha(...)
UserSvc->>UserSvc: addRatingWeightedDelta(deltaWeightedSum, deltaWeight)
UserSvc->>DB: save User and Contribution
UserSvc-->>AppSvc: done
AppSvc-->>Controller: reviewId
Controller-->>Client: 201 Created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/main/java/org/sopt/poti/domain/groupbuy/entity/GroupBuyPost.java`:
- Around line 207-220: The addRating currently computes a simple arithmetic mean
using ratingSum and ratingCount; to implement EMA change addRating(int score) to
first call validateScore(score) then update ratingAvg using the EMA formula
(ratingAvg = alpha * score + (1 - alpha) * ratingAvg) with a defined alpha
(e.g., a private static final double ALPHA = ...), handling the initial case
when ratingCount==0 by setting ratingAvg = score; you can still increment
ratingCount and optionally keep ratingSum for legacy purposes, but replace the
arithmetic-average calculation with the EMA update in the addRating method and
leave validateScore unchanged.
- Around line 72-79: Existing rows in the group_buy_posts table will not get the
new NOT NULL defaults, so create and run an explicit migration/backfill to set
rating_avg=0.0, rating_sum=0, rating_count=0 for all existing records before
deploying the GroupBuyPost field changes: implement a SQL migration (e.g.,
Flyway/V1__backfill_group_buy_ratings.sql or equivalent) that runs UPDATE
group_buy_posts SET rating_avg = 0.0, rating_sum = 0, rating_count = 0 WHERE
rating_avg IS NULL OR rating_sum IS NULL OR rating_count IS NULL, then verify
and only after that apply the schema change that makes ratingAvg, ratingSum,
ratingCount non-nullable in the GroupBuyPost entity.
In `@src/main/java/org/sopt/poti/domain/user/entity/User.java`:
- Around line 59-63: The User entity declares ratingSum and ratingCount but they
are never used; either remove these unused fields to avoid adding unnecessary DB
columns (delete ratingSum and ratingCount from class and update DB
migration/schema) or, if intended to track user ratings like GroupBuyPost, add
explicit usages: implement methods in User such as addRating(long delta) /
incrementRatingCount() / getAverageRating() and ensure service/repository code
updates these fields (referencing the User class and its ratingSum/ratingCount
symbols) so they are actually maintained; choose one approach and apply
corresponding schema migration changes.
🧹 Nitpick comments (1)
src/main/java/org/sopt/poti/domain/user/entity/User.java (1)
127-140: 부동소수점 비교와 입력 유효성 검증에 대해 검토가 필요해요.몇 가지 엣지 케이스 고려해보면 좋을 것 같아요:
부동소수점 정밀도 이슈:
ratingWeightTotal <= 0.0비교가 여러 번의 delta 연산 후 부동소수점 오차로 인해 미세하게 양수(예:0.0000000001)가 되면 의도치 않게 0으로 나누는 상황이 발생할 수 있어요. 아주 작은 양수에 대한 임계값(threshold) 비교를 고려해볼 수 있어요.입력 유효성:
deltaWeightedSum이나deltaWeight가NaN또는Infinity일 경우 데이터가 오염될 수 있어요.♻️ 개선 제안
public void addRatingWeightedDelta(double deltaWeightedSum, double deltaWeight) { + if (Double.isNaN(deltaWeightedSum) || Double.isInfinite(deltaWeightedSum) || + Double.isNaN(deltaWeight) || Double.isInfinite(deltaWeight)) { + throw new IllegalArgumentException("Invalid delta values"); + } + this.ratingWeightedSum += deltaWeightedSum; this.ratingWeightTotal += deltaWeight; - if (this.ratingWeightTotal <= 0.0) { + // 부동소수점 오차를 고려한 임계값 비교 + if (this.ratingWeightTotal < 1e-9) { this.ratingWeightedSum = 0.0; this.ratingWeightTotal = 0.0; this.ratingAvg = 0.0; return; } double avg = this.ratingWeightedSum / this.ratingWeightTotal; this.ratingAvg = Math.round(avg * 10) / 10.0; }
| @Column(name = "rating_sum", nullable = false) | ||
| private long ratingSum = 0L; | ||
|
|
||
| @Column(name = "rating_count", nullable = false) | ||
| private int ratingCount = 0; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ratingSum and ratingCount usages across the codebase
echo "=== Searching for ratingSum usages ==="
rg -n --type=java 'ratingSum' -C 2
echo ""
echo "=== Searching for ratingCount usages ==="
rg -n --type=java 'ratingCount' -C 2Repository: team-poti/POTI-SERVER
Length of output: 3467
ratingSum과 ratingCount 필드가 User 엔티티 내에서 사용되고 있지 않아요.
이 두 필드는 선언만 되어 있고 User 클래스의 어떤 메서드에서도 실제로 사용되지 않고 있네요. 흥미롭게도 GroupBuyPost 엔티티에는 동일한 필드들이 있고 여기서는 적극적으로 사용 중입니다 (rating 계산에서 합계와 개수를 누적하는 형태).
혹시 다른 레이어(서비스, 리포지토리)에서 이 필드들을 사용 중이라면 괜찮지만, 엔티티 단에서 전혀 사용되지 않는다면 불필요한 DB 컬럼 추가가 될 수 있습니다. 한 번 확인해 보시는 게 좋을 것 같아요.
🤖 Prompt for AI Agents
In `@src/main/java/org/sopt/poti/domain/user/entity/User.java` around lines 59 -
63, The User entity declares ratingSum and ratingCount but they are never used;
either remove these unused fields to avoid adding unnecessary DB columns (delete
ratingSum and ratingCount from class and update DB migration/schema) or, if
intended to track user ratings like GroupBuyPost, add explicit usages: implement
methods in User such as addRating(long delta) / incrementRatingCount() /
getAverageRating() and ensure service/repository code updates these fields
(referencing the User class and its ratingSum/ratingCount symbols) so they are
actually maintained; choose one approach and apply corresponding schema
migration changes.
…to refact/#79/add-review-ema # Conflicts: # src/main/java/org/sopt/poti/domain/participation/service/ParticipationService.java
|
@PBEM22 믿지마세요 |
📌 관련 이슈
✨ 변경 사항
📸 테스트 증명 (필수)
📚 리뷰어 참고 사항
✅ 체크리스트
Summary by CodeRabbit
새로운 기능
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.